Use ESM by jablko · Pull Request #1621 · mdx-js/mdx

jablko · 2021-08-28T16:57:34Z

I think this is a prerequisite of upgrading to hast-util-to-estree v2? Otherwise I get the following error, because hast-util-to-estree is pure ESM, which require() doesn't support:

 FAIL  test/index.test.js
  ● Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/en/ecmascript-modules for how to enable it.
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /home/nottheoilrig/mdx/node_modules/hast-util-to-estree/index.js:58
    import {stringify as commas} from 'comma-separated-tokens'
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

    > 1 | const toEstree = require('hast-util-to-estree')
        |                  ^
      2 | const walk = require('estree-walker').walk
      3 | const periscopic = require('periscopic')
      4 | const estreeToJs = require('./estree-to-js')

      at Runtime.createScriptFromCode (../../node_modules/jest-runtime/build/index.js:1350:14)
      at Object.<anonymous> (mdx-hast-to-jsx.js:1:18)

To replace require() with import I ran the following, plus manual fix-ups:

$ sed --in-place '
  s/\(const\|var\) \(.*\) = require(\(.*\))/import \2 from \3/
  s/module.exports = require(\(.*\))/export * from \1/
  s/module.exports = /export default /
' $(
    git ls-files \
      docs \
      packages/loader \
      packages/mdx \
      packages/parcel-plugin-mdx \
      packages/vue-loader \
  )

Depends on jestjs/jest#11790
Depends on webpack/webpack#14077

vercel · 2021-08-28T16:57:38Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mdx/mdx/5CJ94n8dabCW7aGRbychj2ouQ4G9
✅ Preview: https://mdx-git-fork-jablko-patch-3-mdx.vercel.app

jablko · 2021-08-28T17:12:20Z

.eslintrc.yml

-      import:
-        resolver:
-          typescript: null
+      import/resolver:
+        typescript:
+          project: packages/*/types


So eslint-import-resolver-typescript can find @mdx-js/mdx.Options:

✘ https://google.com/#q=import%2Fnamed Options not found in '@mdx-js/mdx' packages/runtime/types/index.d.ts:4:9 2 | 3 | import {FunctionComponent} from 'react' > 4 | import {Options} from '@mdx-js/mdx' | ^ 5 | import {ComponentsProp} from '@mdx-js/react' 6 | 7 | /**

jablko · 2021-08-28T17:12:53Z

packages/loader/package.json

-    "test-api": "jest test",
-    "test-coverage": "jest test --coverage",
+    "test-api": "NODE_OPTIONS=--experimental-vm-modules jest test",
+    "test-coverage": "NODE_OPTIONS=--experimental-vm-modules jest test --coverage",


--experimental-vm-modules for tests written in ESM.

jablko · 2021-08-28T17:13:10Z

packages/loader/package.json

-    "webpack": "^4.0.0"
+    "webpack": "^5.51.1"


Upgrade to webpack 5 for webpack/loader-runner#40, although it's still missing webpack/webpack#14077.

jablko · 2021-08-28T17:13:23Z

packages/loader/test/index.test.js

-      context: __dirname,
+      context: fileURLToPath(new URL('.', import.meta.url)),


No __filename or __dirname in ESM.

jablko · 2021-08-28T17:14:23Z

packages/loader/test/index.test.js

-        // Webpack 5:
-        // resolve(
-        //   {source: fs.readFileSync(path.join(__dirname, '..', 'dist', 'main.js'), 'utf8')}
-        // )
-        resolve(stats.toJson().modules.find(m => m.name === filePath))
+        resolve({
+          source: fs.readFileSync(
+            new URL('../dist/main.js', import.meta.url),
+            'utf8'
+          )
+        })


Upgrade to webpack 5 with the help of the existing comments.

jablko · 2021-08-28T17:16:06Z

packages/mdx/types/index.d.ts


 import {Plugin, Compiler, Processor} from 'unified'

-declare namespace mdx {


Update types correspondingly.

jablko · 2021-08-28T17:16:27Z

packages/parcel-plugin-mdx/src/index.js

-  bundler.addAssetType('mdx', require.resolve('./MDXAsset.js'))
+import {createRequire} from 'module'
+
+export default function (bundler) {
+  bundler.addAssetType(
+    'mdx',
+    createRequire(import.meta.url).resolve('./MDXAsset.js')


createRequire() works, until Jest supports import.meta.resolve().

jablko · 2021-08-28T17:17:29Z

packages/react/package.json

  ],
  "license": "MIT",
-  "main": "dist/mdx-react.js",
+  "main": "dist/mdx-react.cjs",


"type": "module" implies that dist/mdx-react.js is ESM. dist/mdx-react.cjs is identical but explicitly CJS. This avoids:

FAIL test/index.test.js ● Test suite failed to run SyntaxError: The requested module '@mdx-js/react' does not provide an export named 'MDXProvider' at Runtime.linkAndEvaluateModule (../../node_modules/jest-runtime/build/index.js:669:5) at TestScheduler.scheduleTests (../../node_modules/@jest/core/build/TestScheduler.js:333:13) at runJest (../../node_modules/@jest/core/build/runJest.js:387:19) at _run10000 (../../node_modules/@jest/core/build/cli/index.js:408:7) at runCLI (../../node_modules/@jest/core/build/cli/index.js:261:3)

jablko · 2021-08-28T17:17:51Z

packages/vue-loader/test/test.js

+import {mount} from '@vue/test-utils'
+import remarkSlug from 'remark-slug'

-// See `loader`’s tests for how to upgrade these to webpack 5.


Upgrade to webpack 5, same as packages/loader.

jablko · 2021-08-28T17:59:08Z

FYI, current CI failure is pending jestjs/jest#11790.

ChristianMurphy · 2021-08-28T18:28:47Z

Thanks @jablko!
ESM support is of interest and a goal for the project.

A related project, XDM, has ESM support https://github.com/wooorm/xdm you may want to check it out.
It also includes workarounds for WebPack's lack of ESM loader support https://github.com/wooorm/xdm#webpack (though ideally ESM should be natively supported upstream), and native TypeScript support.

There's been discussion of MDX and XDM merging back together, which would likely bring ESM support into MDX.
Which does leave things in a bit of a lurch.
This PR and the potential merge with XDM are both large changes which would significantly overlap. 🤔
It may be worth waiting a bit to see where that goes.

In the meantime, could you expand a bit on what features/fixes from hast-util-to-estree v2 you are looking for?
May be able to offer some pointers while MDX and XDM get sorted out.

wooorm · 2021-10-06T08:00:21Z

Thanks for your work on this Jack.
You may have seen that I since cleaned the repo, ported xdm back into it, and am hard at work working on the new website (https://v2.mdxjs.com). Part of that effort was also ESM, hence I’ll close this.
While the website is not done, and the code has not been released, I would appreciate to hear your thoughts on them if you have any!

jablko · 2021-10-06T14:09:48Z

This is fantastic and exciting, many thanks for your tremendous work!

vercel bot temporarily deployed to Preview August 28, 2021 16:57 Inactive

jablko commented Aug 28, 2021

View reviewed changes

ChristianMurphy added 🌊 blocked/upstream This cannot progress before something external happens first 🗄 area/interface This affects the public interface 🙉 open/needs-info This needs some more info labels Aug 28, 2021

jablko force-pushed the patch-3 branch from 4871fbb to faf908d Compare September 5, 2021 16:07

vercel bot temporarily deployed to Preview September 5, 2021 16:07 Inactive

Use ESM

773dc9e

jablko force-pushed the patch-3 branch from faf908d to 8489368 Compare September 5, 2021 16:20

vercel bot temporarily deployed to Preview September 5, 2021 16:20 Inactive

WIP

fe18728

jablko force-pushed the patch-3 branch from 8489368 to fe18728 Compare September 5, 2021 16:54

vercel bot temporarily deployed to Preview September 5, 2021 16:54 Inactive

wooorm closed this Oct 6, 2021

wooorm removed 🌊 blocked/upstream This cannot progress before something external happens first 🙉 open/needs-info This needs some more info labels Oct 6, 2021

		context: __dirname,
		context: fileURLToPath(new URL('.', import.meta.url)),


		import {Plugin, Compiler, Processor} from 'unified'

		declare namespace mdx {

Uh oh!

Conversation

jablko commented Aug 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Aug 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jablko commented Aug 28, 2021

Uh oh!

ChristianMurphy commented Aug 28, 2021

Uh oh!

wooorm commented Oct 6, 2021

Uh oh!

jablko commented Oct 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

jablko commented Aug 28, 2021 •

edited

Loading

vercel bot commented Aug 28, 2021 •

edited

Loading